Skip to content

Reject stale legacy SAA schedule-to-close timers#10917

Merged
fretz12 merged 4 commits into
feature/activity-operator-cmdsfrom
fredtzeng/saa-stale-legacy-s2c
Jul 9, 2026
Merged

Reject stale legacy SAA schedule-to-close timers#10917
fretz12 merged 4 commits into
feature/activity-operator-cmdsfrom
fredtzeng/saa-stale-legacy-s2c

Conversation

@fretz12

@fretz12 fretz12 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What changed?

Updated SAA ScheduleToClose timer validation to require an exact stamp match. Legacy zero-stamp timers are now only accepted while the activity’s current ScheduleToClose stamp is also zero.

Why?

A stale pre-stamp ScheduleToClose timer could still time out an activity after update-options had extended the timeout and emitted a newer stamped timer.
Matching stamps exactly prevents old timer tasks from remaining valid after the activity advances to a newer ScheduleToClose stamp.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

fretz12 added 3 commits July 2, 2026 19:26
When a running activity has a deferred reset with keep_paused=true, unpause should clear only ResetKeepPaused while preserving the pending reset.
Validate ScheduleToClose timer tasks by exact stamp match so legacy zero-stamp tasks are only valid while the activity stamp is still zero.
@fretz12
fretz12 marked this pull request as ready for review July 6, 2026 03:44
@fretz12
fretz12 requested review from a team as code owners July 6, 2026 03:44
@fretz12
fretz12 requested review from dandavison and spkane31 July 6, 2026 03:44
Comment thread tests/activity_standalone_test.go Outdated
// a newer stamped ScheduleToClose timer.
func (s *standaloneActivityTestSuite) TestUpdateOptionsInvalidatesLegacyScheduleToClose() {
t := s.T()
env := s.newTestEnv(testcore.WithFxOptions(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're going to have issues when we try to merge this PR because the testcore.WithFxOptions has been removed. Can this be a unit test in the chasm/lib/activity instead of here? Would that involve too much use of mocks that it's not worth the test?

cc @stephanos

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I converted it to a unit test; the core logic still gets valid coverage.

Base automatically changed from fredtzeng/saa-unpause-after-reset to feature/activity-operator-cmds July 8, 2026 21:07
@fretz12
fretz12 merged commit fec4f4e into feature/activity-operator-cmds Jul 9, 2026
44 checks passed
@fretz12
fretz12 deleted the fredtzeng/saa-stale-legacy-s2c branch July 9, 2026 16:08
fretz12 added a commit that referenced this pull request Jul 10, 2026
## What changed?
Updated SAA ScheduleToClose timer validation to require an exact stamp
match. Legacy zero-stamp timers are now only accepted while the
activity’s current ScheduleToClose stamp is also zero.

## Why?
A stale pre-stamp ScheduleToClose timer could still time out an activity
after update-options had extended the timeout and emitted a newer
stamped timer.
Matching stamps exactly prevents old timer tasks from remaining valid
after the activity advances to a newer ScheduleToClose stamp.

## How did you test it?
- [X] built
- [X] run locally and tested manually
- [ ] covered by existing tests
- [ ] added new unit test(s)
- [X] added new functional test(s)
dandavison pushed a commit that referenced this pull request Jul 18, 2026
Implements Pause, Unpause, Reset, and UpdateActivityExecutionOptions
RPCs for standalone CHASM activities. Adds matching frontend and
history service handlers in chasm/lib/activity that dispatch to
either the workflow-activity path or the standalone-activity path
based on the presence of a workflow ID.

Also adds PAUSE_REQUESTED and RESET_REQUESTED state machine states
for standalone activities, the original_options snapshot for
restore-on-reset semantics, the activity-options merge helpers
(common/activityoptions), and the supporting proto schema changes
(start_request_id, sdk_name, sdk_version, etc.).

Squashed PRs:
  #9693  RPC boilerplate and code generation
  #9850  Implement UpdateActivityExecutionOptions
  #9851  Implement Pause/UnpauseActivityExecution for SAA
  #9852  Implement ResetActivityExecution for SAA
  #10001 Pause returns FailedPrecondition when already paused
  #10025 Validator function fixes
  #10265 Replace PauseState flag with PAUSE_REQUESTED state
  #10364 Add RESET_REQUESTED state to standalone activity
  #10745 Add support for updating start_delay via UpdateActivityOptions
  #10913 Preserve SAA retry backoff across unpause re-dispatch
  #10914 Fix SAA unpause for keep-paused reset
  #10917 Reject stale legacy SAA schedule-to-close timers
  #10920 Preserve NextRetryDelay across unrelated activity option updates
  #10921 Validate merged SAA retry policy updates
  #10923 Fix SAA reset attempt dispatch time reporting
  #11010 Use default retry policy when updating with nil
  #11020 Fix: de-duplicate pause even if activity is unpaused
  #11013 Fix: reject restore_original when a standalone activity has no original options snapshot
  #11032 Reject unsupported and empty update_mask paths in UpdateActivityExecutionOptions
  #11030 Fix: describe only provides heartbeat/failure when opted-in
  #11016 Preserve pending activity next retry delay
  #11068 Add SAA operator APIs to DC redirection whitelist
  #10954 Remove reset_heartbeats field in ResetActivityExecutionRequest
  #11087 Fix standalone activity heartbeat flag precedence
  #11117 Validation to reject negative activity jitter
  #11120 Fix bogus StartToCloseLatency for non-running standalone activities
  #11097 Fix task token invalidation after resetting an activity
  #11067 SAA bug fix: update start delay while paused
  #11123 SAA: cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants